Skip to content

fix(test): [Queue Instrumentation 39] Remove stale Kafka container before startup#5347

Open
adinauer wants to merge 6 commits intofix/kafka-interceptor-lifecycle-tokenfrom
fix/kafka-system-test-stale-container
Open

fix(test): [Queue Instrumentation 39] Remove stale Kafka container before startup#5347
adinauer wants to merge 6 commits intofix/kafka-interceptor-lifecycle-tokenfrom
fix/kafka-system-test-stale-container

Conversation

@adinauer
Copy link
Copy Markdown
Member

@adinauer adinauer commented Apr 29, 2026

PR Stack (Queue Instrumentation)


📜 Description

Clean up stale Kafka system-test containers before starting the local broker.

SystemTestRunner.start_kafka_broker() now force-removes the named container up front, even when the current runner instance did not start the previous broker. stop_kafka_broker() stays ownership-aware and still only tears down brokers started by the current run.

💡 Motivation and Context

A review comment on the original Kafka console sample PR pointed out that interrupted runs could leave behind a stopped container named sentry-java-system-test-kafka. The runner tried to clean that up before startup, but the cleanup path returned early whenever kafka_started_by_runner was false, so the next docker run --name ... failed with a container-name conflict.

This change fixes the stale-container path without changing the reuse behavior for an already running broker on localhost:9092.

I also looked into replacing Redpanda with the official Apache Kafka image. That is possible, but it is not a drop-in swap because the current startup command is Redpanda-specific, so I left that as a separate follow-up.

💚 How did you test it?

  • Ran ./gradlew spotlessApply apiDump
  • Ran python3 -m py_compile test/system-test-runner.py
  • Ran a mocked Python verification that start_kafka_broker() now issues docker rm -f sentry-java-system-test-kafka before docker run

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

If we want the most official broker image for these tests, switch the runner to apache/kafka in a separate PR.

#skip-changelog

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

Always remove the named Kafka system-test container before starting a new broker. This avoids docker name conflicts after crashed or interrupted runs while still keeping stop_kafka_broker ownership-aware for reused brokers.

Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Apr 29, 2026
@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 29, 2026

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.37.1 (1) release

⚙️ sentry-android Build Distribution Settings

@adinauer adinauer marked this pull request as ready for review April 29, 2026 09:12
When SENTRY_AUTO_INIT=true with the OTel agent, Sentry is initialized
early by SentryAutoConfigurationCustomizerProvider before Spring Boot
loads application-kafka.properties. Without the env var, queue tracing
stays disabled and OTel messaging spans are not mapped to
queue.publish/queue.process ops, causing
KafkaOtelCoexistenceSystemTest to fail.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant